Skip to content

Batch 4 part 2 — kayak-deploy: digest-verified paired releases (S7, D2) - #190

Merged
mousebrains merged 14 commits into
mainfrom
b4-paired-release
Jun 13, 2026
Merged

Batch 4 part 2 — kayak-deploy: digest-verified paired releases (S7, D2)#190
mousebrains merged 14 commits into
mainfrom
b4-paired-release

Conversation

@mousebrains

Copy link
Copy Markdown
Owner

Second slice of finishing-plan Batch 4: the paired-release activation orchestrator (deploy/kayak-deploy.sh, installed as /usr/local/sbin/kayak-deploy by the 4C runbook). Nothing on the current host consumes it yet — fully inert until the 4C cutover re-points nginx and installs it.

Design (plan §S7, trimmed per accepted decision D2)

  • Trust anchor: full 40-hex commit SHAs only (branches/tags refused), each verified reachable from its repo's protected branch in a scratch bare clone; SHA-256 digests of the built wheel, dataset snapshot (git archive), and host-config fingerprint recorded in release.json. Deployer version is documented (KAYAK_DEPLOY_VERSION=1), not negotiated.
  • Release = self-contained dir under /opt/kayak/releases/<id>/: venv + wheel, read-only dataset snapshot (contract-validated with the staged engine before anything else), non-secret runtime-config.json, docroot. Release id = digest(wheel ⊕ dataset SHA ⊕ host-config fingerprint) — a host-config-only change is a distinct release.
  • Activation: maintenance flag → stop consumers (configurable unit list) → sqlite3 .backupmigrate → all-or-nothing sync-metadata → build into the release → atomic symlink switch (ln + rename) → optional health probe → restart → maintenance off. Any failure: symlink back to the previous release, restore the DB backup if mutation began, restart units, preserve the scratch backup for manual recovery.
  • --stage-only stops before any system mutation — the test mode and the recommended first run on any host.

Tests

Ref-shape rejection, missing-conf failure, unreachable-SHA rejection (fast), plus a slow end-to-end staging run using this repo's HEAD as the engine and the fixture dataset as a local git repo — asserts the venv, snapshot, runtime config, digest manifest, and that stage-only never creates current. bash-3.2-compatible (suite runs on the dev Mac); prod needs only python3+pip3+git.

The activation phase gets its first real exercise in the Batch 4C clean-VM rehearsal (the planned virgin install), before the prod cutover.

Gate: ruff + format, pytest (1718 + the slow staging test), mypy, shellcheck — green.

🤖 Generated with Claude Code

mousebrains and others added 4 commits June 12, 2026 17:14
… activation (S7/4B, D2)

The activation orchestrator the plan's S7 calls for, trimmed per
decision D2: trust = a full 40-hex commit SHA reachable from each
repo's protected branch (verified in a scratch bare clone) plus
SHA-256 digests of the built wheel, dataset snapshot, and host-config
fingerprint, all recorded in release.json — no signature machinery,
and the deployer version is documented rather than negotiated.

A release under $KAYAK_DEPLOY_ROOT/releases/<id>/ is self-contained:
venv with the engine wheel, read-only dataset snapshot (git archive),
contract-validated before anything else, non-secret runtime-config,
docroot. The release id derives from wheel digest + dataset SHA +
host-config fingerprint, so a host-config-only change is a distinct
release. Activation: maintenance flag -> stop consumers -> sqlite
.backup -> migrate -> all-or-nothing sync -> build into the release ->
atomic symlink switch (ln + rename) -> optional health probe -> restart
-> maintenance off; any failure rolls back the symlink, restores the
DB backup if mutation began, restarts units, and preserves the scratch
backup for manual recovery.

--stage-only stops before any system mutation and is both the test mode
and the recommended first run on a new host. Tests: ref-shape rejection
(short/branch/tag/non-hex), missing-conf failure, unreachable-SHA
rejection, and a slow end-to-end staging run (this repo's HEAD as the
engine + the fixture dataset as a local git repo) asserting the venv,
snapshot, runtime config, and digest manifest. bash-3.2-compatible so
the suite runs on the dev Mac; prod needs only python3+pip3+git
(hatchling fetched at wheel build).

Activation is exercised on a real host by the Batch 4C clean-VM
rehearsal (the planned virgin install), which also re-points nginx at
$ROOT/current/docroot via the unit/vhost renderers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…detached HEAD

git rev-parse --abbrev-ref HEAD returns the literal 'HEAD' on CI's
detached checkout, so ENGINE_BRANCH=HEAD made the deployer's
single-branch clone fail (exit 128) in both branch-dependent tests.
The module fixture now pushes HEAD into a scratch bare repo under a
known 'test-main' branch, which works identically attached or detached.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…kout is shallow too

git push from a shallow repository is refused, so the bare-repo push
fixture failed on CI just like --abbrev-ref had. git archive HEAD works
regardless of shallow/detached state; the staging test exercises the
deployer's mechanics (clone, wheel build, snapshot, digests), not
provenance, so a tree-identical single-commit repo is the right
fixture.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…umed

Third CI-environment difference: the test job's PATH leads with a
uv-managed venv whose python has no pip module, so 'python3 -m pip
wheel' failed. The deployer now needs only the stdlib venv/ensurepip:
a scratch venv supplies pip for the wheel build and the release venv
bootstraps its own, with an early capability check (clear error +
KAYAK_DEPLOY_PYTHON escape hatch). Host requirement documented:
python3-venv on Debian, no system pip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mousebrains

mousebrains commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

Adversarial review findings for PR #190 at current head 1c7d7cb26bde57835eb05db4d434b6cba154d0b0:

  1. P1 — The new deploy path never applies reach geometry/gradient sidecars. deploy/kayak-deploy.sh only runs levels sync-metadata and then levels build during activation (deploy/kayak-deploy.sh:284-289). That skips the current deployer's scripts/import_metadata.py --geom-only / --gradient-only steps (scripts/deploy.sh:199-216). This is not redundant: sync-metadata deliberately excludes reach.geom and reach.gradient_profile, and scripts/import_metadata.py documents itself as the only live-DB writer for those sidecar columns (scripts/import_metadata.py:4-10, src/kayak/db/metadata_csv.py:3-8). The build then generates public reaches-geom.json from Reach.geom in the DB (src/kayak/web/build/geojson.py:35-36, src/kayak/web/build/deploy.py:546-551). A paired dataset release that changes reaches.json or reaches-gradient.json can validate and activate while still serving stale geometry/gradient data from the old DB. Fix: make sidecar apply part of the packaged deploy surface, e.g. a levels import-metadata command or equivalent packaged function, and run it after sync-metadata and before build, with rollback covered by the existing DB backup.

  2. P1 — Release identity/reuse ignores /etc/kayak/env, even though staging emits runtime config from it. The script sources /etc/kayak/env (deploy/kayak-deploy.sh:79-81) and emits runtime-config.json from the resulting environment (deploy/kayak-deploy.sh:198-200). But the release id only uses wheel digest + dataset SHA + host.yaml fingerprint (deploy/kayak-deploy.sh:172-181), and an existing release dir skips re-staging entirely (deploy/kayak-deploy.sh:184-186). Changing a non-secret runtime config input in /etc/kayak/env while deploying the same engine/dataset refs can reuse the old release directory with stale runtime-config.json and docroot. Fix: include the generated runtime config digest, or the relevant env/config fingerprint, in release identity and release.json verification; refuse or regenerate an existing release on mismatch.

  3. P1 — Activation does not actually quiesce in-flight writers before mutating the DB. The default KAYAK_UNITS contains only timers (deploy/kayak-deploy.sh:87), and activation stops exactly those units (deploy/kayak-deploy.sh:271-274). Stopping a systemd timer does not stop an already-running oneshot service. If kayak-pipeline.service, kayak-decimate.service, or backup services are already active, they can keep reading/writing while the deployer takes PRE_BACKUP, runs migrations, syncs metadata, and builds (deploy/kayak-deploy.sh:276-289). On activation failure, rollback restores the pre-activation DB snapshot (deploy/kayak-deploy.sh:255-259), which can discard writes made by those still-running services after the backup. Existing ops paths explicitly stop services as well as timers (docs/operations.md:135-138, scripts/db_push.sh:100-105). Fix: split stop/start sets or expand the stop set to include the corresponding .service units, and wait until they are inactive before the backup/mutation phase.

  4. P2 — A first activation rollback can leave the failed release as current. PREV_TARGET is only set when $ROOT/current already exists (deploy/kayak-deploy.sh:237-240). After the symlink switch (deploy/kayak-deploy.sh:291-293), a failing health check triggers rollback, but rollback only rewrites current when PREV_TARGET is non-empty (deploy/kayak-deploy.sh:261-264). On a virgin host, there is no previous target, so the failed release remains pointed to by $ROOT/current; then rollback restarts units and removes maintenance (deploy/kayak-deploy.sh:265-266). That violates the script's “Any failure rolls back” activation contract. Fix: track whether the switch happened; if there was no previous target, remove $ROOT/current and leave the host in maintenance or otherwise refuse to restart consumers. Add a non-stage-only rollback test for this path.

  5. P2 — The release is not fully digest-verified because dependency resolution happens at deploy time. The engine wheel is built with pip wheel --no-deps (deploy/kayak-deploy.sh:161-162), then the release venv installs that wheel with normal pip dependency resolution (deploy/kayak-deploy.sh:190-191). The manifest records the engine wheel and dataset/config digests (deploy/kayak-deploy.sh:203-214), but not the installed dependency wheels. Since pyproject.toml uses ranges such as SQLAlchemy>=2.0, requests>=2.33, etc. (pyproject.toml:11-24), the same reviewed engine SHA and dataset SHA can produce different runtime code on different days, and branch protection/CI no longer describes what prod executed. Fix: build from the locked resolver output, install with hashes or a prebuilt wheelhouse, and include the dependency lock/wheelhouse digest in release.json/RELEASE_ID.

Verification run locally on the current PR head:

  • bash -n deploy/kayak-deploy.sh passed
  • git diff --check origin/main...HEAD passed
  • env UV_CACHE_DIR=/private/tmp/kayak-pr190-uv-cache HOME=/private/tmp/kayak-pr190-home uv run --extra dev pytest -q tests/test_scripts/test_kayak_deploy.py passed: 3 passed, 1 deselected
  • gh pr checks 190 is green
  • shellcheck deploy/kayak-deploy.sh only reports SC1091 for the dynamic /etc/kayak/env fallback source

… identity, locked deps, virgin rollback

All five PR #190 findings:

P1 sidecars: new packaged kayak.db.sidecars + 'levels import-metadata'
(the geom/gradient apply sync-metadata deliberately excludes;
scripts/import_metadata.py now delegates to the same functions);
activation runs it after sync, before build, so a sidecar-only dataset
release reaches the DB. db/sidecars.py joins the writer-boundary
ALLOWLIST with rationale.

P1 quiesce: activation stops timers AND their services, then waits (max
120s) for the service set to drain before the pre-mutation backup — a
timer stop alone leaves an in-flight oneshot writing under the backup/
migrate/sync.

P1 identity: the non-secret runtime config is emitted during staging
from the live host env and its digest joins the release id, so an
/etc/kayak/env change mints a NEW release instead of reusing a stale
runtime-config/docroot. Manifest records it.

P2 locked deps: the engine commit now carries requirements-prod.lock
(hash-pinned uv export, drift-checked in CI); release and staging venvs
install with --require-hashes + the wheel with --no-deps, and the lock
digest joins the release id — the same engine SHA installs identical
bytes on any day.

P2 virgin rollback: the symlink switch is tracked; a first-activation
failure removes 'current', keeps maintenance mode, and does NOT restart
consumers (there is no prior release to serve).

Tests: import-metadata unit suite (both/only/rollback/partial/
malformed); staging test asserts the new digests and that a SITE_URL
change yields a different release id.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

All five findings addressed in 6f746c7:

P1 — sidecar apply: new packaged kayak.db.sidecars + levels import-metadata command (default applies both sidecars; --geom-only/--gradient-only are a mutually-exclusive group; unmatched ids roll back and fail unless --allow-missing-reaches). scripts/import_metadata.py now delegates to the same packaged functions — one implementation. Activation runs it between sync-metadata and build, covered by the pre-activation DB backup. db/sidecars.py joins the writer-boundary ALLOWLIST with rationale.

P1 — quiesce: activation stops timers and their corresponding services, then waits (≤120 s) for the whole service set to drain before the backup; a still-active service aborts rather than mutating under it.

P1 — release identity: staging now installs the engine into the scratch toolchain, validates the dataset, and emits the non-secret runtime config from the live host env before the release id is derived — the config digest (and the dependency-lock digest) join the id and the manifest, so an /etc/kayak/env change mints a new release; stale-reuse is structurally impossible.

P2 — locked deps: the engine now carries requirements-prod.lock (hash-pinned uv export, drift-checked in CI against uv.lock); both staging and release venvs install with --require-hashes plus the wheel --no-deps. Same engine SHA ⇒ identical dependency bytes, any day, recorded in the manifest.

P2 — virgin rollback: the symlink switch is tracked; a first-activation failure removes current, keeps maintenance mode, and does not restart consumers — a half-activated first install neither serves nor writes.

Tests: new import-metadata unit suite (both/only/rollback/partial/malformed), and the staging test now asserts the lock+config digests and that a SITE_URL change yields a different release id. Full gate green: pytest 1723 + the slow staging run (now two stagings, ~83 s), mypy, shellcheck, wheel-smoke, lock-drift check in sync.

@mousebrains

Copy link
Copy Markdown
Owner Author

Re-review of PR #190 at current head 6f746c7ccff18764e3036c74ace913488aa2410f after the previous fixes:

  1. P1 — Activation switches a release symlink that the web stack does not serve. The new deployer builds into $RELEASE_DIR/docroot and atomically switches $ROOT/current (deploy/kayak-deploy.sh:360-367), but this PR does not update the checked-in serving config to use that path. The nginx common snippet still has root /home/pat/public_html; (conf/snippets/levels-common.conf:30), and the FPM pool open_basedir still only allows /home/pat/public_html plus /etc/kayak/runtime-config.json (deploy/kayak-fpm-pool.conf:60). The PR file list also does not touch those config files. Net: kayak-deploy can stage/build a new release, switch /opt/kayak/current, run HEALTH_URL against the old public docroot, and report success while users still get the previous deployment. Fix either needs the config/install side of the symlink cutover in the same PR (nginx root/aliases, FPM open_basedir, PHP config path as needed), or the deployer needs to keep publishing to the existing /home/pat/public_html surface.

  2. P1 — Runtime config is neither installed where PHP reads it nor valid as a release-local config. The deployer emits config with DATASET_DIR="$SCRATCH/dataset" OUTPUT_DIR="$SCRATCH/dataset" (deploy/kayak-deploy.sh:199-201) and only copies that JSON into the release dir (deploy/kayak-deploy.sh:227). emit-config dumps the full KayakConfig (src/kayak/cli/emit_config.py:95), including output_dir and dataset_dir (src/kayak/config.py:132-177), so the copied JSON records scratch paths that are removed by the cleanup trap. PHP still reads /etc/kayak/runtime-config.json unless KAYAK_CONFIG_PATH is set (src/kayak/web/php/includes/config.php:47-52), and the current deploy path installs that file through the root wrapper that also merges /etc/kayak/secrets.env (scripts/deploy.sh:235-236). This new deployer does neither. So config changes do not reach PHP; and if the web stack were later pointed at $ROOT/current/runtime-config.json, it would read deleted scratch paths and miss the wrapper’s secret merge. Fix: render the final config with release paths, install it via /usr/local/sbin/kayak-install-runtime-config, or explicitly change PHP/FPM to consume a release-local config and preserve the secret-merge/open_basedir invariants.

  3. P2 — The “locked deps” fix still leaves the build backend mutable and network-resolved at deploy time. The deployer runs pip wheel --no-deps before installing the hash-locked requirements (deploy/kayak-deploy.sh:162-188). Because pyproject.toml declares an unpinned PEP 517 build backend (pyproject.toml:1-3), pip build isolation resolves and executes hatchling from PyPI during staging. That backend is not covered by requirements-prod.lock, LOCK_SHA, or --require-hashes. I reproduced this locally: the slow stage-only test fails without network while trying to resolve /simple/hatchling/; the same test passes with network enabled. For a digest-verified deployer, a mutable build backend can execute code and change the produced wheel outside the reviewed engine SHA. Fix: lock/hash the build backend too and build with --no-build-isolation from the preinstalled locked toolchain, or build/install from a prebuilt wheelhouse whose digest participates in the release id.

  4. P2 — First-activation rollback is still incomplete for failures before the symlink switch. The new rollback special-case only keeps maintenance on when SWITCHED=1 and there is no PREV_TARGET (deploy/kayak-deploy.sh:280-300). On a virgin host, if migrate, sync-metadata, import-metadata, or build fails before SWITCHED is set at deploy/kayak-deploy.sh:367, rollback falls through to starting consumers and removing maintenance (deploy/kayak-deploy.sh:301-306) even though there is still no $ROOT/current. The prior failure mode after a bad health check is fixed; the pre-switch first-activation failure mode is not. Fix: when PREV_TARGET is empty, any activation failure after quiescing should leave maintenance on and avoid restarting consumers, regardless of whether the symlink switch happened.

Validation I ran locally:

  • bash -n deploy/kayak-deploy.sh passed
  • git diff --check origin/main...HEAD passed
  • ruff check src/kayak/cli/import_metadata.py src/kayak/db/sidecars.py tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py passed
  • pytest -q tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py --override-ini=addopts= passed with network enabled: 9 passed in 96.20s
  • The same focused pytest command without network failed in test_stage_only_builds_verified_release while pip tried to resolve hatchling from PyPI; that is finding 3 above
  • shellcheck deploy/kayak-deploy.sh only reports the existing SC1091 for the dynamic /etc/kayak/env source
  • GitHub PR checks are green

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review for updated PR #190 at head 6f746c7ccff18764e3036c74ace913488aa2410f, with live-server context included.

Live context I checked:

  • Current live kayak_python checkout is still 6da8dc5931dacb74e0eb8ed84e6189ad60ecc868; current live kayak_data checkout is 5dcf55da03ffa427f79529aea7c62319c573d7de.
  • /opt/kayak does not exist on this host today; the current public docroot is /home/pat/public_html.
  • PHP currently reads /etc/kayak/runtime-config.json, which is installed as root:www-data 0640; /etc/kayak/secrets.env is root-only. The live FPM pool open_basedir names /etc/kayak/runtime-config.json, not a release-local config path.
  • At review time, all kayak-* services were inactive and the timers were active/waiting. The live DB has 432 reaches, all with geom and gradient_profile; change_request status counts were rejected=1, resolved=5, no pending status row.

Findings:

  1. P1 - The staged runtime config is never activated for PHP, and it bypasses the existing secret-merge boundary. deploy/kayak-deploy.sh emits $SCRATCH/runtime-config.json and copies it into the release (deploy/kayak-deploy.sh:199-202, deploy/kayak-deploy.sh:227), but activation never installs that JSON to /etc/kayak/runtime-config.json and does not set KAYAK_CONFIG_PATH for PHP-FPM. The PHP reader defaults to /etc/kayak/runtime-config.json unless that env var is present (src/kayak/web/php/includes/config.php:26, src/kayak/web/php/includes/config.php:47-51), and the live FPM config only opens /etc/kayak/runtime-config.json (deploy/kayak-fpm-pool.conf:49-60). The current deploy path explicitly runs levels emit-config --dry-run | sudo -n /usr/local/sbin/kayak-install-runtime-config (scripts/deploy.sh:221-236), and that wrapper is the security boundary that merges root-only /etc/kayak/secrets.env values and installs 0640 root:www-data (deploy/kayak-install-runtime-config.sh:12-14, deploy/kayak-install-runtime-config.sh:95-99). With this PR, a deploy can switch code/docroot while PHP continues reading the old /etc/kayak/runtime-config.json; if the release-local JSON is later used directly, it still lacks the wrapper's root-only secret merge. Fix: make runtime-config activation explicit and atomic in kayak-deploy.sh (or change FPM/open_basedir to a release-local KAYAK_CONFIG_PATH with the same wrapper/ownership/secret semantics), and health-check the PHP config path actually being served.

  2. P1 - CONFIG_SHA and RELEASE_ID are contaminated by scratch paths, so identical inputs are not repeatable and the copied config contains dead paths. levels emit-config serializes the whole KayakConfig via cfg.model_dump(...) (src/kayak/cli/emit_config.py:95), including output_dir, dataset_dir, gauge_metadata_cache, and osmb_dir (src/kayak/config.py:132-176). The deployer generates that config with DATASET_DIR="$SCRATCH/dataset" OUTPUT_DIR="$SCRATCH/dataset" (deploy/kayak-deploy.sh:199-201), then includes its digest in the release id (deploy/kayak-deploy.sh:202-215) and deletes $SCRATCH on exit (deploy/kayak-deploy.sh:105-110). On this host, the stage-only artifacts from the focused test run contained random /tmp/... paths in the copied runtime-config.json. That means same engine ref + same dataset ref + same host env can mint a different release id every run, defeating the "same reviewed inputs => same staged release" property; the release also retains config paths that do not exist after cleanup. The current test only asserts that changing SITE_URL changes the release id (tests/test_scripts/test_kayak_deploy.py:187-202); it does not assert same-input stability or absence of scratch paths. Fix: either emit the release config with final release paths, or split the digest surface so path-local staging fields are excluded from release identity; add a same-input stage test and a no-scratch-path assertion for runtime-config.json.

  3. P2 - The release directory now retains operational tokens that the code calls "non-secret". The config model includes ntfy_topic and all hc_* healthcheck URLs (src/kayak/config.py:200, src/kayak/config.py:226-240), and build_config_data() serializes any non-None values (src/kayak/cli/emit_config.py:95). kayak-deploy.sh copies that JSON into each retained release directory (deploy/kayak-deploy.sh:227) after creating the release tree as the deploy user (deploy/kayak-deploy.sh:222). The existing /etc/kayak/runtime-config.json path is intentionally installed as root:www-data 0640 by the wrapper; retained release copies widen the lifetime and ownership boundary for healthcheck ping URLs/notification topics, and stage-only runs on the live host also write those values into /tmp artifacts. Fix: do not label this JSON non-secret unless those fields are filtered, or keep the sensitive runtime config only under the existing /etc/kayak wrapper path with the same ownership and cleanup expectations.

Verification performed on the current PR head:

  • GitHub PR checks are green for 6f746c7.
  • bash -n deploy/kayak-deploy.sh passed.
  • git diff --check origin/main...HEAD passed.
  • ruff check passed on the new/changed Python files I targeted.
  • mypy src/kayak/cli/import_metadata.py src/kayak/db/sidecars.py passed.
  • pytest tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py --override-ini=addopts= passed: 9 passed in 108.00s.
  • shellcheck deploy/kayak-deploy.sh only reported the expected dynamic-source SC1091 for /etc/kayak/env.

mousebrains and others added 3 commits June 12, 2026 18:36
…ty, locked build backend, virgin pre-switch rollback

Second-round PR #190 findings:

P1 serving gap: activation now REFUSES unless deploy.env sets
SERVING_CUTOVER=yes — the flag the Batch 4C install/migration runbook
sets when nginx root + FPM open_basedir actually point at
$ROOT/current. Until then stage-only works and scripts/deploy.sh stays
the deploy path; the deployer can no longer report success while users
are served the legacy docroot.

P1 config activation + secret boundary: activation installs the
canonical /etc/kayak/runtime-config.json exactly the way deploy.sh
does — emit-config --dry-run piped into the root
kayak-install-runtime-config wrapper (root-only secrets merged, 0640
root:www-data), emitted with FINAL release paths.

P1 identity stability + token retention: the release id's config
digest is computed over a NORMALIZED view (staging-local path fields
and ntfy/hc_* operational tokens excluded, keys sorted), so identical
inputs mint the identical release id, the release-retained copy holds
no dead scratch paths and no notification/healthcheck tokens, and the
canonical secret-merged config lives only under /etc/kayak.

P2 build backend: requirements-build.lock (hash-pinned hatchling chain
via uv pip compile) is preinstalled into the build venv and the wheel
builds with --no-build-isolation — no network resolution and no
unpinned backend code at deploy time; its digest joins the release id
and manifest.

P2 virgin rollback: with no previous release, ANY activation failure —
before or after the symlink switch — now leaves the host in
maintenance with consumers stopped ('current' removed only if the
switch happened).

Tests: same-input staging mints the same release id (reuse path), the
retained runtime-config has no scratch paths / no ntfy_topic / no hc_*
/ no token value, and the SITE_URL-change different-id assertion stays.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Zip entries carry checkout mtimes, so the same engine SHA hashed
differently per clone — breaking the same-inputs => same-release-id
property the re-review demanded. hatchling honors SOURCE_DATE_EPOCH;
pin it to the commit's own timestamp (deterministic per SHA).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Its engine default is BASE_DIR-relative, which resolves inside the
scratch build venv — the one remaining nondeterministic field in the
release-identity digest (and a dead path in the retained copy). The DB
location is host-runtime shape; PHP reads it from the canonical
wrapper-installed /etc/kayak/runtime-config.json.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

All seven findings from both second-round passes addressed (e8f04f0):

P1 — serving gap: activation now refuses unless deploy.env sets SERVING_CUTOVER=yes — the flag the Batch 4C install/migration runbook sets when nginx root + FPM open_basedir actually point at $ROOT/current. Until then --stage-only works and scripts/deploy.sh remains the deploy path; the deployer can no longer report success while users are served the legacy docroot.

P1 — config activation + secret boundary: activation installs the canonical /etc/kayak/runtime-config.json exactly the way deploy.sh does — emit-config --dry-run piped into the root kayak-install-runtime-config wrapper (root-only secret merge, 0640 root:www-data), emitted with final release paths.

P1 — identity stability + token retention: the release id's config digest is computed over a normalized view — staging-local path fields (dataset_dir, output_dir, database_url, caches) and operational tokens (ntfy_topic, hc_*) excluded, keys sorted. Two more nondeterminism sources fell out of testing this properly: wheel zip mtimes (fixed with SOURCE_DATE_EPOCH = the commit's own timestamp, honored by hatchling) and the BASE_DIR-relative database_url default resolving inside the scratch venv. The staging test now stages three times: same inputs ⇒ same release id (reuse path hit), SITE_URL change ⇒ new id, and the retained config is asserted to contain no scratch paths, no ntfy_topic/hc_* keys, and not the seeded token value.

P2 — build backend: new requirements-build.lock (hash-pinned hatchling chain via uv pip compile --generate-hashes) preinstalled into the build venv; the wheel builds with --no-build-isolation — no network resolution and no unpinned backend code at deploy time; its digest joins the release id and manifest.

P2 — virgin rollback (pre-switch): with no previous release, any activation failure — before or after the symlink switch — now leaves the host in maintenance with consumers stopped (current removed only if the switch happened).

Gate: pytest 1723 + the slow triple-staging run (~2 min), mypy, ruff, shellcheck — green.

@mousebrains

Copy link
Copy Markdown
Owner Author

Re-review at e8f04f0 after the latest updates. CI is green now.

Findings:

  • P1: Rollback does not restore /etc/kayak/runtime-config.json after a failed activation. The deployer installs the canonical runtime config before the final switch/health phase (deploy/kayak-deploy.sh:417-420), then switches current and runs the optional health check (deploy/kayak-deploy.sh:422-429). If ln, mv, or the health check fails, rollback() restores the DB backup and previous symlink (deploy/kayak-deploy.sh:332-350), but it never snapshots or restores the previous runtime config. PHP treats that JSON as its production source of truth (src/kayak/web/php/includes/config.php:26 and src/kayak/web/php/includes/config.php:47-52), so the rollback can leave the old release and restored DB running with the failed release config. That is not atomic: feature flags, site identity/license/mail settings, and any path fields generated from the failed release can stay live even though current was put back. Please snapshot the existing runtime config before installing the new one and restore it from rollback() once that file has been mutated. I would also add an activation-path regression test with an existing current, a successful wrapper install, and a failing health check, asserting that DB, symlink, and runtime config all return to the previous release.

Local validation run: bash -n deploy/kayak-deploy.sh, git diff --check origin/main...HEAD, focused ruff check, and focused uv run --extra dev pytest -q tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py --override-ini=addopts=. The focused pytest run passed (9 passed).

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial re-review for PR #190 at head e8f04f0a8d783ffd5fb9d8db1eb0d69434a892f3, including live-server context. CI was still running when requested; by my final check all GitHub checks were green for this head.

Live context checked:

  • Live kayak_python is still 6da8dc5931dacb74e0eb8ed84e6189ad60ecc868; live kayak_data is still 5dcf55da03ffa427f79529aea7c62319c573d7de.
  • /opt/kayak does not exist on this host today; current public docroot is /home/pat/public_html.
  • /etc/kayak/runtime-config.json is root:www-data 0640; /etc/kayak/secrets.env exists and is root:www-data 0600.
  • Live kayak-* services were inactive and timers waiting at review time.
  • Live DB is WAL mode, owned by pat; change_request status counts were rejected=1, resolved=5, and all 432 reaches had both geom and gradient_profile.

Findings:

  1. P1 - The release-retained config can still capture root-only secrets when the deployer runs with the privileges it otherwise requires. Activation now directly calls systemd and /usr/local/sbin/kayak-install-runtime-config (deploy/kayak-deploy.sh:366-372, deploy/kayak-deploy.sh:417-420), and the example config points at root's deploy-key setup, so the practical activation mode is root. Under that mode, kayak.config reads /etc/kayak/secrets.env whenever it is readable (src/kayak/config.py:80-82), emit-config explicitly unwraps SecretStr values (src/kayak/cli/emit_config.py:100-101), and turnstile_secret is a SecretStr field (src/kayak/config.py:223). The new normalization only drops path fields, database_*, ntfy_topic, and hc_* (deploy/kayak-deploy.sh:222-234); it does not drop turnstile_secret. That normalized JSON is copied into every release dir (deploy/kayak-deploy.sh:260). On this live host, /etc/kayak/secrets.env exists and is root-only, so a root-run deploy can persist the Turnstile secret into retained release artifacts, despite the comment saying the canonical secret-merged config lives only under /etc/kayak. Fix: make the release-retained normalized config impossible to populate from root-only secrets, e.g. run that staging emit with secrets unreadable or explicitly drop all secret fields before copying; add a deploy test with TURNSTILE_SECRET set, not just NTFY_TOPIC.

  2. P1 - Rollback does not restore /etc/kayak/runtime-config.json after installing the new config. The script installs the canonical runtime config before switching current and before the health check (deploy/kayak-deploy.sh:417-429). If ln -s, mv, or the HEALTH_URL probe fails, the rollback restores the DB backup and previous symlink (deploy/kayak-deploy.sh:328-355), but it never restores the previous /etc/kayak/runtime-config.json. That leaves the previous release served with the failed release's config: site identity, editor/mail flags, DB path, Turnstile fields, etc. The old deploy path did not have this split-brain because code/docroot/config moved as one live tree. Fix: back up the current runtime config before installing the new one and restore it on any rollback after line 420, or defer/redo the install in a sequence with an explicit config rollback test for a failing post-switch health check.

  3. P1 - The privilege model is still ambiguous for this live host. The checked-in and live systemd services run the app as User=pat (systemd/kayak-pipeline.service:8, systemd/kayak-decimate.service:8, systemd/kayak-status.service:7), and the live DB/docroot are owned by pat. kayak-deploy.sh, however, directly performs privileged orchestration and then runs the app CLI as the invoking user: migrate, sync-metadata, import-metadata, build, and emit-config are all invoked without dropping privileges (deploy/kayak-deploy.sh:393-420). If the script is run as pat, the systemctl/config-install/root paths fail; if run as root, DB writes and build outputs happen as root against resources the rest of the live system expects pat to own. With WAL-mode SQLite, root-created sidecars are an avoidable production footgun. Fix: define/enforce one model: either a root orchestrator that runs app-level commands as pat and reserves root only for systemctl/config install, or a pat orchestrator with narrow sudo calls for the privileged steps.

  4. P2 - requirements-build.lock is not drift-checked in CI. The deployer now relies on requirements-build.lock for the PEP 517 backend (deploy/kayak-deploy.sh:162-181), but CI only drift-checks requirements-prod.lock (.github/workflows/ci.yml:289-299). I do not see any check that regenerates/compares requirements-build.lock from pyproject.toml's build-system requirements. A future build-backend change can merge with a stale build lock and only fail at deploy time, or keep using an unintended backend chain. Fix: add a CI drift check for requirements-build.lock with the exact generation command documented in the file.

Validation performed locally on this head:

  • GitHub checks: all green by final check.
  • bash -n deploy/kayak-deploy.sh passed.
  • git diff --check origin/main...HEAD passed.
  • Targeted ruff check passed.
  • mypy src/kayak/cli/import_metadata.py src/kayak/db/sidecars.py passed.
  • pytest tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py --override-ini=addopts= passed: 9 passed in 141.13s.
  • shellcheck deploy/kayak-deploy.sh only reported the expected dynamic-source SC1091 for /etc/kayak/env.

…-lock CI check, activation test

PR #190 third-round findings (both passes):

P1 config rollback: activation snapshots /etc/kayak/runtime-config.json
before installing the new one and rollback() restores it on any
post-install failure (failed switch/health), so a rollback never
leaves the old release running with the failed release's config.

P1 secret retention: the normalized release-retained config now drops
any secret/password/token-shaped field (root-run staging emit unwraps
SecretStr like turnstile_secret) in addition to path/ntfy/hc_* fields.

P1 privilege model: one orchestrator — root for systemctl + the config
wrapper; DB/build steps (backup/migrate/sync/import/build) run as
KAYAK_APP_USER via runuser so root never creates app-owned-resource WAL
sidecars. Read-only scratch staging stays as the orchestrator (the
secret filter, not the uid, guards the retained copy). Required-when-root
KAYAK_APP_USER documented; the root/runuser env propagation is validated
in the 4C clean-VM rehearsal.

P2 build-lock drift: requirements-build.lock is regenerated from
pyproject.toml [build-system].requires and diffed in CI, so a backend
change can't merge stale and only fail at deploy time.

Tests: systemctl + config-installer are parameterized so a new slow
activation test runs the full path unprivileged — first release
activates, a second release with a failing health check rolls back the
symlink, DB, AND runtime config to release 1. The staging test now also
asserts TURNSTILE_SECRET never reaches the retained config.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

All third-round findings addressed in 65b7f29 (both passes):

P1 — config rollback (both reviewers): activation snapshots /etc/kayak/runtime-config.json before installing the new one, and rollback() restores it on any post-install failure (failed ln/mv/health). A rollback now returns symlink, DB, and runtime config to the previous release together. New slow activation test proves it: first release activates, a second release with a failing health probe rolls all three back to release 1.

P1 — secret retention: the normalized release-retained config drops any secret/password/token-shaped field (a root-run staging emit unwraps SecretStr like turnstile_secret), on top of the path/ntfy/hc_* filter. The staging test now seeds TURNSTILE_SECRET and asserts neither the key nor its value survives.

P1 — privilege model: one orchestrator. Root performs systemctl + the root config wrapper; every step that writes persistent app-owned state (backup/migrate/sync/import/build) runs as KAYAK_APP_USER via runuser, so root never creates app-owned-resource WAL sidecars. Read-only scratch staging stays as the orchestrator (the secret filter, not the uid, guards the retained copy). KAYAK_APP_USER is required-when-root and documented; the root/runuser env propagation is validated in the 4C clean-VM rehearsal (it needs real root+systemd — same framing as the rest of the activation phase).

P2 — build-lock drift: requirements-build.lock is now regenerated from pyproject.toml [build-system].requires and diffed in CI, so a backend change can't merge stale and only surface at deploy time.

Gate: pytest 1723 + the slow deploy suite (10 tests, ~3.5 min incl. the new activation rollback test), mypy, ruff, shellcheck, both lock drift checks — green.

@mousebrains

Copy link
Copy Markdown
Owner Author

Re-review at 65b7f2959328ffa0b02ec591efcf0418f7776e84 after the latest updates. CI is green by final check.

Findings:

  1. P1 - Root-run activation cannot create the pre-activation DB backup as KAYAK_APP_USER. The script documents root as the normal activation orchestrator and drops persistent DB/docroot writes through run_app (deploy/kayak-deploy.sh:105-124). But SCRATCH comes from mktemp -d (deploy/kayak-deploy.sh:143), which creates a private 0700 directory owned by the orchestrator. In the root path, the script then creates $SCRATCH/app owned by KAYAK_APP_USER, points PRE_BACKUP under it, and runs sqlite3 .backup via run_app (deploy/kayak-deploy.sh:448-456). The app user still cannot traverse the root-owned scratch parent, so a real root-run activation will fail at the first backup before migrations/build even start. The current activation test does not catch this because it runs the whole deployer as one unprivileged uid with stubs (tests/test_scripts/test_kayak_deploy.py:314-337), never through runuser. Put the backup in an app-user-created/app-user-traversable temp directory, or make the relevant scratch path deliberately traversable by the app user, and add a regression that exercises the uid boundary.

  2. P1 - Rollback still restores SQLite as root, violating the new app-user ownership model on the failure path. The new model says DB writes run as KAYAK_APP_USER to avoid root-owned WAL sidecars (deploy/kayak-deploy.sh:105-116), and the forward path now mostly uses run_app. But if any post-mutation step fails, rollback calls plain sqlite3 "$DB_PATH" .restore ... as the orchestrator (deploy/kayak-deploy.sh:390-394), then restarts consumers and removes maintenance (deploy/kayak-deploy.sh:411-412). SQLite is forced into WAL mode by the app (src/kayak/db/engine.py:23-27), and the checked-in services run as User=pat (systemd/kayak-pipeline.service:6-9, systemd/kayak-decimate.service:6-9), so a failed deploy can still leave root-created or root-mutated DB sidecars precisely when the script is trying to recover production. Restore the DB via run_app as well, with the backup path readable by that user, and extend the activation rollback test to assert the restore command crosses the same app-user boundary as backup/migrate/sync/build.

Local validation run on this head: bash -n deploy/kayak-deploy.sh, git diff --check origin/main...HEAD, targeted ruff check, targeted mypy, shellcheck deploy/kayak-deploy.sh (only the expected dynamic /etc/kayak/env SC1091), and focused uv run --extra dev pytest -q tests/test_cli/test_import_metadata.py tests/test_scripts/test_kayak_deploy.py --override-ini=addopts=. The focused pytest run passed: 10 passed in 233.51s.

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial re-review for PR #190 at head 65b7f2959328ffa0b02ec591efcf0418f7776e84, including live-server context. CI was still running when requested; by my final check all GitHub checks were green for this head.

Live context checked:

  • Live kayak_python is still 6da8dc5931dacb74e0eb8ed84e6189ad60ecc868; live kayak_data is still 5dcf55da03ffa427f79529aea7c62319c573d7de.
  • /opt/kayak does not exist on this host today; the current public docroot is /home/pat/public_html.
  • /etc/kayak/runtime-config.json is root:www-data 0640; /etc/kayak/secrets.env is root-only.
  • Live app resources are owned by pat: /home/pat/DB and /home/pat/public_html are pat:pat, and the live SQLite DB is in WAL mode.
  • Live kayak-* services were inactive and timers waiting at review time. The checked-in and live app services still run User=pat from /home/pat/kayak with /home/pat/.venv/bin/levels.

Findings:

  1. P1 - Activation can still restart old-code systemd consumers after a paired release. The new SERVING_CUTOVER gate only talks about nginx/FPM serving config pointing at $ROOT/current (deploy/kayak-deploy.sh:346-356, deploy/deploy.env.example:31-36). It does not cover the app consumers the deployer stops and starts around DB mutation (deploy/kayak-deploy.sh:420-445, deploy/kayak-deploy.sh:504-506). On this live host, systemctl cat matches the checked-in units: kayak-pipeline.service and kayak-decimate.service have WorkingDirectory=/home/pat/kayak and ExecStart=/home/pat/.venv/bin/levels ... (systemd/kayak-pipeline.service:9, systemd/kayak-pipeline.service:19, systemd/kayak-decimate.service:9, systemd/kayak-decimate.service:19), and kayak-status.service is the same shape (systemd/kayak-status.service:8, systemd/kayak-status.service:21). So after a successful release activation, the web docroot/config can point at the new release while the next pipeline/decimate/status run uses the old checkout and old venv against the migrated DB and newly synced dataset. That breaks the paired-release invariant in the live deployment model. Fix: make the cutover gate cover systemd consumers too, or have kayak-deploy verify the units/drop-ins it is about to restart actually execute $ROOT/current/venv/bin/levels with the intended release environment before activation can proceed.

  2. P1 - The root/app-user DB boundary is still broken in the actual root-run path. The script creates SCRATCH with mktemp -d (deploy/kayak-deploy.sh:143), then in root mode creates only $SCRATCH/app as KAYAK_APP_USER and points PRE_BACKUP inside it (deploy/kayak-deploy.sh:449-456). A mktemp -d parent is mode 0700 for the creating root user, so run_app sqlite3 ... ".backup '$PRE_BACKUP'" runs as pat but cannot traverse the root-owned scratch parent to create the backup. That means the normal live mode documented in deploy/deploy.env.example:13-19 fails before mutation. Separately, if a later failure happens after MUTATED=1, rollback still restores the DB with plain root sqlite3 (deploy/kayak-deploy.sh:390-393), reintroducing the root-owned WAL/SHM footgun the new run_app model is meant to avoid. The new activation test does not cover this branch: it runs unprivileged with stubbed systemctl/config installer and never sets KAYAK_APP_USER (tests/test_scripts/test_kayak_deploy.py:314-337), so run_app() is just a pass-through (deploy/kayak-deploy.sh:118-128). Fix: make the backup path traversable/writable by the app user without widening secrets-bearing scratch paths, run DB restore through the same app-user boundary, and add a root/runuser activation regression or a test hook that exercises the root branch.

  3. P2 - Reusing an existing release only verifies the wheel, not the digest-verified release contents. The release manifest records dataset_tar_sha256, requirements_lock_sha256, build_lock_sha256, and runtime_config_sha256 (deploy/kayak-deploy.sh:313-324), but if $RELEASE_DIR/release.json already exists the script skips staging (deploy/kayak-deploy.sh:301-303) and then validates only the staged wheel digest (deploy/kayak-deploy.sh:331-337). Activation subsequently trusts $RELEASE_DIR/venv/bin/levels and $RELEASE_DIR/dataset for migrations, metadata sync, sidecar import, and build (deploy/kayak-deploy.sh:458-475). A stale/corrupted/reused release directory can therefore mutate the live DB from unverified dataset files while still passing the only activation digest check. For a deployer whose safety claim is digest-verified paired releases, reuse needs to fail closed. Fix: verify the full release.json against the current calculated inputs and verify the retained artifacts needed for activation, or restage into a fresh temp release and atomically replace on mismatch; add a regression that corrupts an existing release dataset/config/lock and proves activation refuses it.

Validation performed locally on this head:

  • GitHub checks: all green by final check.
  • bash -n deploy/kayak-deploy.sh passed.
  • git diff --check origin/main...HEAD passed.
  • Targeted ruff check passed.
  • mypy src/kayak/cli/import_metadata.py src/kayak/db/sidecars.py passed.
  • shellcheck deploy/kayak-deploy.sh only reported the expected dynamic-source SC1091 for /etc/kayak/env.
  • Focused pytest initially failed because /tmp is a 964 MB tmpfs and pip hit Errno 28; rerun with TMPDIR on the main filesystem passed: 10 passed in 261.63s.

…ase verify (4th round)

PR #190 fourth-round findings (both passes):

P1 app-user DB boundary: the pre-activation backup now lands in an
APP-OWNED scratch dir (runuser mktemp -d), since the orchestrator's
0700 mktemp -d is untraversable by the app user; the rollback DB
restore runs through run_app too, so neither path recreates the
root-owned WAL/SHM footgun. run_app's privilege decision and the
runuser command are overridable (KAYAK_PRIVILEGED / KAYAK_RUNUSER) so
the activation test exercises the privileged branch with a same-user
shim — it now asserts backup/migrate/sync/import/build AND the rollback
restore all cross the app-user boundary.

P1 consumer cutover: SERVING_CUTOVER now means the whole host is cut
over, and activation verifies each levels-running consumer unit's
ExecStart points at $ROOT/current before mutating — otherwise the next
pipeline/decimate/status run would execute the old checkout against the
migrated DB. Gate doc + deploy.env.example updated.

P2 reused-release verify: a reused release dir is now fully verified —
every digestable retained artifact (wheel, prod lock, runtime-config)
against both the manifest and the recomputed inputs, the dataset tar
digest, and a venv liveness check; the dataset is re-extracted from the
freshly-verified tar. Any mismatch fails closed (test corrupts a
retained lock and asserts refusal).

Also fixes an EXIT-trap gotcha the new app-scratch introduced: cleanup
ended on a failing test expression, leaking exit 1 into stage-only
(caught by the staging test).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

All fourth-round findings addressed in 97a2987 (both passes):

P1 — app-user DB boundary (real bug): the pre-activation backup now lands in an app-owned scratch dir (runuser mktemp -d), since the orchestrator's mktemp -d is 0700 root — untraversable by the app user, so the previous $SCRATCH/app approach would have failed at the first backup on a real root run. The rollback DB .restore runs through run_app too, so neither the forward nor the failure path recreates the root-owned WAL/SHM footgun. run_app's privilege decision and the runuser command are now overridable (KAYAK_PRIVILEGED/KAYAK_RUNUSER), so the activation test runs the privileged branch with a same-user shim and asserts backup/migrate/sync/import/build and the rollback restore all cross the boundary.

P1 — consumer cutover: SERVING_CUTOVER now means the whole host is cut over, and activation verifies each levels-running consumer unit's ExecStart points at $ROOT/current before mutating — closing the gap where the next pipeline/decimate/status run would execute the old checkout against the migrated DB. Gate doc + deploy.env.example updated.

P2 — reused-release verify: a reused release dir is now fully verified — every digestable retained artifact (wheel, prod lock, runtime-config) against both the manifest and the recomputed inputs, plus the dataset-tar digest and a venv liveness check; the dataset is re-extracted from the freshly-verified tar. Any mismatch fails closed (new test corrupts a retained lock and asserts refusal).

Bonus the new tests caught: adding the app-scratch introduced an EXIT-trap gotcha (cleanup ended on a failing [ … ] chain, leaking exit 1 into stage-only) — fixed.

Gate: pytest 1723 + the full deploy suite (11 tests now, incl. privileged-mode activation rollback + corrupted-reuse refusal, ~4 min), mypy, ruff, shellcheck, both lock drift checks — green. (Local runs need TMPDIR off the 964 MB /tmp tmpfs, as you noted.)

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review — PR #190 (kayak-deploy, S7/D2)

Reviewed against the live host's actual configuration, not just the diff. The design is sound and the engineering is careful (atomic switch, app-user DB boundary, hash-locked deps, fail-closed reuse, reproducible wheels). The PR is also genuinely inert today — /usr/local/sbin/kayak-deploy is not installed, there's no /etc/kayak/deploy.env, /opt/kayak doesn't exist, and every consumer unit still runs /home/pat/.venv/bin/levels. So no regression risk to current prod.

The findings below are about what happens when 4C actually points this at the live host. The recurring theme: the tests pass only because they inject into the process env the very values the live host does not supply that way, so the staging/activation env model is effectively unverified against reality.


P1 — SQLITE_PATH won't resolve on this host → activation aborts before any work

kayak-deploy.sh:85-90 sources only /etc/kayak/env, with the comment "The standard host environment (SITE_URL, SQLITE_PATH, …)". On this host that file contains exactly one line: KAYAK_HOME=/home/pat. The real config — SQLITE_PATH, SITE_URL, DATASET_DIR, OUTPUT_DIR, NTFY_TOPIC, HC_* — lives in /home/pat/.config/kayak/.env (0600 pat). The systemd units get it via a second EnvironmentFile=/home/pat/.config/kayak/.env (confirmed in kayak-pipeline.service); the deployer never reads that file.

Consequence at kayak-deploy.sh:462-466:

DB_PATH="${SQLITE_PATH:-}"
if [ -z "$DB_PATH" ] && [ -r /etc/kayak/env ]; then
    DB_PATH="$(. /etc/kayak/env >/dev/null 2>&1; echo "${SQLITE_PATH:-}")"
fi
: "${DB_PATH:?SQLITE_PATH must be set (env or /etc/kayak/env) for activation}"

SQLITE_PATH is in neither the orchestrator env nor /etc/kayak/env, so DB_PATH is empty and activation aborts at the :? guard. test_activation_* only passes because it sets SQLITE_PATH directly in the subprocess env.

Fix options: have the deployer also source ~${KAYAK_APP_USER}/.config/kayak/.env, or have the 4C runbook consolidate SQLITE_PATH/DATASET_DIR/SITE_URL into /etc/kayak/env (and say so in deploy.env.example). Either way the comment at line 85 is currently false for this host.

P1 — staging emit-config (root) needs SITE_URL, which only arrives via the sudo/SUDO_USER path

The live dataset is status: publishable (/home/pat/kayak_data/dataset.yaml), so require_explicit_site_url_for_publishable_dataset makes emit-config fail unless SITE_URL is in os.environ (config.py:330-332, site_url_is_explicitly_configured() reads os.environ). Staging runs emit-config as the orchestrator (kayak-deploy.sh:281, not run_app), i.e. root on a real host.

This works only if the deployer is invoked as sudo kayak-deploy from pat — then SUDO_USER=pat triggers config.py:43-51's fallback to /home/pat/.config/kayak/.env, and load_dotenv puts SITE_URL into os.environ. If kayak-deploy is ever started directly as root (root login, systemd, cron — none of which set SUDO_USER), the fallback can't fire, SITE_URL is absent, and even --stage-only fails for the publishable dataset. The test deliberately sets SUDO_USER="" and instead injects SITE_URL, so this invocation contract is both undocumented and untested. Please document "must be run via sudo from the app user" (or pass SITE_URL explicitly) and ideally assert it early.

P2 — cutover-verification gate's *levels* heuristic misses an engine consumer

The gate (kayak-deploy.sh:440) only verifies units whose ExecStart contains levels:

case "$es" in *levels*) … must contain $ROOT/current … ;; esac

But kayak-audit-gauges.service — which is in the default KAYAK_UNITS — runs /home/pat/.venv/bin/python3 .../scripts/audit_gauges.py (no levels token), so it gets stopped/started but its ExecStart is never checked against $ROOT/current. After 4C it could keep running the old checkout against the freshly-migrated DB and the gate would still pass. (It's read-mostly, so low blast radius — but the heuristic is a leaky proxy for "engine consumer.") Same class: kayak-recap/-healthcheck/-heartbeat are shell scripts under /home/pat/kayak/ and aren't in KAYAK_UNITS at all. Consider keying off $ROOT/current presence or an explicit host-script exemption list rather than the levels substring.

P2 — no release pruning; unbounded disk growth

/opt/kayak/releases/<id>/ each carries a full venv. Nothing GCs old releases (the only rm -rf paths are scratch and a restage's dataset/). On the Hetzner VPS this accumulates a venv per deploy with no retention bound. Worth a "keep last N" sweep before this becomes the deploy path — and given rollback relies on PREV_TARGET, you need at least the previous release retained anyway.

P2 — activation test fidelity

test_activation_rolls_back_* stubs runuser with a same-user, env-preserving shim (shift 3; exec "$@"), points HOME at tmp_path, and injects SITE_URL/SQLITE_PATH. That means three real-host behaviors are entirely unexercised: (a) real runuser -u env semantics, (b) config resolution via ~pat/.config/kayak/.env + the SUDO_USER fallback, (c) the bash SQLITE_PATH/SITE_URL sourcing gap (P1 above). The PR acknowledges (a) is deferred to the 4C clean-VM rehearsal; please make that rehearsal explicitly assert (b) and (c) too, since they're the latent failures here.


Verified solid (credit where due)

  • Atomic symlink switch (ln + mv -f), DB .backup before mutation into an app-owned scratch dir, restore-on-failure all through run_app, virgin-host "leave in maintenance" path.
  • Reuse fails closed: full digest re-verification of every retained artifact vs both manifest and recomputed inputs (PYVERIFY), dataset re-extracted from the verified tar.
  • Reproducible wheel via SOURCE_DATE_EPOCH=commit ct; --require-hashes runtime + build-backend locks with CI drift checks; --no-build-isolation so no deploy-time resolution.
  • Secret handling: I enumerated KayakConfigturnstile_secret is the only SecretStr field, and the secret|password|token name filter catches it; the staging test asserts TURNSTILE_SECRET's value never reaches the retained config. (Caveat: the filter is name-based, so a future SecretStr named e.g. *_key/*_credential would leak — consider filtering by SecretStr type at emit time instead.)
  • SERVING_CUTOVER gate correctly prevents reporting success while users are served the legacy docroot.
  • Python side is clean: db/sidecars.py preserves the executemany rowcount→unmatched semantics; levels import-metadata rolls back the whole apply on unmatched ids (test-covered), the --geom-only/--gradient-only boolean logic is correct, and the writer-boundary allowlist is updated with rationale.

Nit

  • fetch_and_verify comment says it will "fetch [the ref] explicitly" but there's no explicit fetch — it relies on the --single-branch bare clone already containing all ancestors of the branch tip (which is correct; the comment is just misleading).

Bottom line: mergeable as inert infrastructure, but the two P1s must be reconciled in the 4C runbook before this orchestrator can stage or activate on the live host as currently configured — the config lives in ~pat/.config/kayak/.env, and the deployer's shell only looks at /etc/kayak/env.

— reviewed on the live server (levels.wkcc.org)

mousebrains and others added 3 commits June 12, 2026 23:24
…, type-based secret exclusion

PR #190 live-host review (reviewed against the running server):

P1 host-env model: on the WKCC host /etc/kayak/env holds only KAYAK_HOME;
the real config (SQLITE_PATH/SITE_URL/DATASET_DIR) lives in the app
user's ~/.config/kayak/.env, which the units load via a second
EnvironmentFile=. The deployer now sources BOTH (KAYAK_HOST_ENV +
KAYAK_APP_ENV/the app user's .config, resolved via getent) with set -a
so subprocesses inherit them — so staging emit-config sees SITE_URL and
DB_PATH resolves, run directly as root or via sudo (no SUDO_USER
dependency). The false 'standard host environment' comment is fixed;
SQLITE_PATH/SITE_URL absence fails early and clearly.

P2 cutover gate: replaced the leaky *levels* substring (which missed
kayak-audit-gauges, a python-run engine consumer) with: every consumer
service must run from $ROOT/current unless explicitly in
KAYAK_HOST_UNITS (default: the backup units).

P2 release pruning: keep KAYAK_KEEP_RELEASES (default 5) most-recent
releases after a successful activation, always retaining current +
previous (rollback needs PREV_TARGET); each release carries a full venv
so this bounds VPS disk growth.

Secret exclusion: emit-config gains --exclude-secrets (TYPE-based: drops
every SecretStr field, not a name heuristic), which the deployer uses
for the normalized/retained config so a future secret field can't leak
by name; the name filter stays as a second line of defense. Nit: the
fetch_and_verify comment no longer claims a non-existent explicit fetch.

Tests: the activation test now resolves SITE_URL+SQLITE_PATH from a
KAYAK_HOST_ENV FILE (exercising the real sourcing the live review flagged
as untested), new prune test (3 activations, KEEP=0, asserts the oldest
pruned + current/previous kept), and an emit-config --exclude-secrets
type-based test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The release-activation cutover used the idiom

    ln -s "releases/$RELEASE_ID" "$ROOT/current.new"
    mv -f "$ROOT/current.new" "$ROOT/current"

which is correct only on the FIRST activation, when $ROOT/current does
not yet exist. On every later deploy `current` is a symlink to a
directory, and both GNU and BSD `mv` dereference such a destination:
`mv -f current.new current` moves current.new INTO releases/<old-id>/,
leaving `current` pointed at the OLD release. Build/migrate/sync/
config-install all succeed and the script exits 0 — but nothing serves
the new release. The new prune step then reads `cur` from the stale
symlink, so the freshly-built (now unreferenced) release matches neither
current nor previous and is garbage-collected: a no-op cutover that also
deletes the release it was meant to activate.

Replace both the forward switch and the rollback restore with an
`atomic_relink` helper that repoints the symlink WITHOUT dereferencing
it: GNU `mv -T`/--no-target-directory, BSD/macOS `mv -h`, each an atomic
rename(2). Try GNU, then BSD, then a non-atomic remove+move only if `mv`
supports neither flag.

Caught by test_activation_prunes_old_releases (added with the pruning
feature): three real activations produce three distinct release ids, yet
`current` stayed pinned to release 1 and only release 1 survived on disk.
Now green, alongside the existing rollback activation test (which also
performs a second switch over an existing `current`).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The prod host's /tmp is a ~1 GB tmpfs. kayak-deploy staged into
`mktemp -d` (i.e. /tmp): the wheel build + two venv installs (a few
hundred MB) plus the app-owned pre-activation DB backup (the live DB is
~650 MB) overflow it, and the deploy dies mid-stage with
ENOSPC — exactly when it is about to mutate production. No unit test
could surface this; it took the Batch 4C clean-VM rehearsal (the test
VM mirrors prod's small /tmp tmpfs).

Default the scratch base to KAYAK_DEPLOY_TMPDIR (= $KAYAK_DEPLOY_ROOT/.staging,
always real disk) for BOTH the orchestrator scratch and the app-owned
DB-backup scratch. The app scratch is carved from the same real-disk base
(0755 so the app user can traverse in) and chowned to the app user, instead
of the app user's own `mktemp -d`. Use the portable `mktemp -d <dir>/X...`
template form (`-p` is GNU-only) so the macOS unit tests still pass.

Validated on the clean-VM clone: stage-only AND a full activation (the
650 MB DB backup) both succeed with /tmp at 82% and no TMPDIR set; /tmp
usage stays flat. deploy.env.example documents the var and the
sourced-shell quoting rule for multi-word values.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

What's new since the last review round + how it was validated on real hardware

Head is now a22585b. Three commits landed since the last reviewed state (97a2987), and — more importantly — the whole deployer was exercised end-to-end on a faithful copy of the production host, which is where the two bug fixes below came from. None of that validation is visible in the diff, hence this note.

The clean-VM rehearsal (the main new evidence)

A throwaway VM was built into a faithful current-live copy (same Debian 13.5, same packages, the live DB copy — 432 reaches, the two repos at the live SHAs, real nginx + PHP-FPM + the 14 systemd units serving the site over HTTPS). Then the full live → paired-release cutover was run on it via kayak-deploy:

phase result
--stage-only wheel build + hash-locked deps from PyPI, dataset snapshot + validate, normalized-config digest, fail-closed reuse — green
first activation (cutover) root/runuser app-user boundary (backup/migrate/sync/sidecars/build as the service user), canonical config via the root wrapper, atomic symlink create, serving restored from the release (homepage + DB-backed description.php 200 through the new docroot + open_basedir)
second activation new release id → current moved to it — the exact path the symlink bug broke
rollback failing health check (exit 7) → restored symlink + DB + config to the previous release; serving stayed up
prune KEEP=0 pruned the oldest, kept current + previous

This is real root + real systemd + a real second service user + GNU coreutils — the things the unit tests could only simulate.

Two bugs the rehearsal found (and fixed)

abb8ded — the atomic symlink cutover silently no-ops on every deploy after the first. ln -s … current.new && mv -f current.new current is correct only when current doesn't exist yet. Once current is a symlink to a directory, both GNU and BSD mv dereference it and move current.new into the old release dir, leaving current on the old release — then the new prune step reads cur from the stale symlink and GCs the freshly-built release. (This idiom was marked "Verified solid" in a prior round; it isn't.) Fixed with an atomic_relink helper using the portable "don't treat dest as a directory" flag — GNU mv -T / BSD mv -h, each an atomic rename(2) — used by both the forward switch and the rollback restore. Validated by the second-activation row above (and it confirms the bug would have hit prod, since the clone is GNU coreutils).

a22585b — staging into /tmp overflows the prod tmpfs (ENOSPC mid-deploy). Prod's /tmp is a ~1 GB tmpfs. The deployer staged into mktemp -d (i.e. /tmp); the wheel build + two venv installs plus the app-owned ~650 MB pre-activation DB backup overflow it, and the deploy dies right as it's about to mutate prod. This would have failed the real cutover. Fixed: KAYAK_DEPLOY_TMPDIR (default $KAYAK_DEPLOY_ROOT/.staging, always real disk) for both the orchestrator scratch and the app-owned DB-backup scratch. Validated on the clone — stage-only and a full activation (the 650 MB backup) both succeed with /tmp at 82% and no TMPDIR set; /tmp usage stays flat.

be43a93 is the response to the previous live-host review (app-user ~/.config/kayak/.env sourcing so SITE_URL/SQLITE_PATH reach emit-config; cutover gate keying off $ROOT/current rather than the levels substring; prune_releases + KAYAK_KEEP_RELEASES; type-based --exclude-secrets).

Findings deferred to follow-on PRs (not in this PR)

  • kayak-audit-gauges runs a source script (scripts/audit_gauges.py) absent from releases → promote to a levels audit-gauges subcommand. Follow-on PR.
  • The hourly pipeline rebuilds the docroot → it must not write into the immutable release. Decided shape: OUTPUT_DIR/var/cache/kayak/docroot (regenerable cache; nginx serves it; the release holds venv+dataset only). Follow-on, with the 4C unit/vhost renderers (the rehearsal's hand-crafted unit drop-ins + nginx/FPM re-point are their spec).

The cutover itself is still gated behind SERVING_CUTOVER=yes, so this remains inert on the live host until the 4C runbook flips it.

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review, including the current live-server context:

Findings

P1 - Root deployer executes the app user's dotenv as shell

deploy/kayak-deploy.sh:97-116 sources both KAYAK_HOST_ENV and the app user's ~/.config/kayak/.env with set -a in the deployer process. On the live server, the second file is exactly what the units load (EnvironmentFile=-/home/pat/.config/kayak/.env) and it is app-user owned (-rw------- pat pat). systemd treats EnvironmentFile= as data; . "$APP_ENV" is shell execution. In the normal root activation path, any app-user write to that file can execute arbitrary commands as root before the deployer reaches the trust/ref checks, and can also override deploy controls such as ENGINE_REPO, KAYAK_SYSTEMCTL, KAYAK_CONFIG_INSTALLER, KAYAK_UNITS, etc. This turns the service account's runtime config file into a root code-execution surface.

Please parse the needed runtime keys as data with a narrow allowlist, or run the env-loading/config-emission portion entirely under the app user and pass only sanitized values back. Do not source an app-owned dotenv in the root orchestrator.

P1 - The default quiesce/cutover unit set misses a live DB consumer

The live host currently has kayak-healthcheck.timer active, and kayak-healthcheck.service runs /home/pat/kayak/scripts/health-check.sh with ReadWritePaths=/home/pat/DB. That is a DB consumer and, after 4C, also a release-layout consumer. But the deployer default at deploy/kayak-deploy.sh:123 and the example at deploy/deploy.env.example:22 omit kayak-healthcheck.timer. The cutover gate and quiesce loops only inspect $KAYAK_UNITS (deploy/kayak-deploy.sh:500-521, 607-628), so with the shipped defaults the deployer can migrate/sync/restore the DB while the hourly healthcheck remains runnable from the legacy checkout, and it will not fail the "all consumers point at $ROOT/current" gate.

This is not just theoretical: systemd/install.service.sh installs kayak-healthcheck.timer, deploy/SETUP.md documents it, and it is active on this server. Either derive the set from installed/active kayak-* timers and require explicit host-unit exemptions, or make the default/example complete for every DB/release consumer. At minimum, kayak-healthcheck.timer needs to be in the default stop/start/gate set.

P2 - Reusing an existing release mutates the release before stage-only/gates/quiesce

In the reuse branch, deploy/kayak-deploy.sh:386-395 does rm -rf "$RELEASE_DIR/dataset" and re-extracts the dataset before the --stage-only exit, before SERVING_CUTOVER is checked, and before consumers are stopped. If $RELEASE_DIR is the current release, a harmless-looking stage-only rehearsal with the same refs can temporarily remove the live release's dataset while timers/web/manual consumers are still running. That violates the "stage-only stops before system mutation" promise and the release immutability model.

Reuse should be read-only until activation quiesces consumers. Verify against scratch artifacts without deleting the existing release's dataset, or restage into a new temp dir and atomically replace only when it is safe.

Verification

  • shellcheck --severity=warning deploy/kayak-deploy.sh passed.
  • pytest tests/test_scripts/test_kayak_deploy.py -q passed only the non-slow subset (3 passed, 3 deselected).
  • pytest tests/test_scripts/test_kayak_deploy.py -m '' -q failed locally on this live server in the two slow activation tests with Error: database or disk is full during the SQLite backup. This host's /tmp is a 964M tmpfs, and the tests force KAYAK_DEPLOY_ROOT under pytest's /tmp, bypassing the PR's new real-disk default. CI is green, but the full deploy-script test module is not runnable as-is on this prod-shaped host.

@mousebrains

Copy link
Copy Markdown
Owner Author

Adversarial review — PR #190 (kayak-deploy paired-release orchestrator)

Reviewed deploy/kayak-deploy.sh + supporting Python against the running WKCC host (levels, the live editable-install tree). This PR has already been through ~6 review rounds; I focused on what only a check against the real production environment can surface. Verdict: no merge blockers. The activation path is inert today (refused unless SERVING_CUTOVER=yes, which no host sets yet), so this lands safely. Everything below is hardening that bites at or after the Batch 4C cutover, when this script first mutates prod for real.

What I verified against the live host (claims that hold up ✅)

  • /tmp ENOSPC fix is real and correct. /tmp is a tmpfs sized 964 MB; the live DB is 647 MB (/home/pat/DB/kayak.db). Staging on $ROOT/.staging (real disk; / has 19 GB free) is necessary, not theoretical.
  • Two-env-file model matches this host. /etc/kayak/env really does contain only KAYAK_HOME=/home/pat; SQLITE_PATH/SITE_URL/DATASET_DIR/OUTPUT_DIR live in pat's ~/.config/kayak/.env. Sourcing both (KAYAK_HOST_ENV + the app .env via getent) is the right call here.
  • Build can't escape the release sandbox. I checked the worry that levels build would write the live public_html: kayak.config calls load_dotenv(..., override=False) (config.py:59,82), so the inline OUTPUT_DIR=$RELEASE_DIR/docroot wins over the .env value. Sandbox holds. ✅
  • runuser is present at /usr/sbin/runuser (root-reachable PATH); sqlite3, git, curl, getent, python3 + stdlib venv/ensurepip all present; the config wrapper /usr/local/sbin/kayak-install-runtime-config exists.
  • atomic_relink is correct on this host: bash 5.2, GNU coreutils → mv -fT succeeds, so the "cutover that survives an existing current" fix works as intended.
  • Disk growth is bounded/modest. requirements-prod.lock is 30 packages with no geo/sci heavies (geo/dev are extras, excluded by uv export --no-dev), so a release venv is far lighter than the 991 MB dev venv. 5+current+previous releases fit comfortably in 19 GB.

Findings

F1 — Medium (defense-in-depth gap): the default KAYAK_UNITS / cutover gate is blind to several live DB-reading & engine consumers.
The default list covers 8 units, but the live host runs engine/DB consumers that are neither in KAYAK_UNITS nor in the KAYAK_HOST_UNITS exemption:

  • kayak-healthcheck.servicescripts/health-check.sh reads the DB via sqlite3 (several queries); highest-frequency non-pipeline unit.
  • kayak-heartbeat.service → reads ${KAYAK_HOME}/DB/kayak.db via sqlite3.
  • kayak-recap.service → runs python3 scripts/recap.py from the old checkout (/home/pat/kayak/...).
  • kayak-config-drift.service → runs from the old checkout.

The cutover-verification loop iterates only over $KAYAK_UNITS, so it can't catch a consumer you forgot to list. Two consequences: (a) health-check/heartbeat read the DB concurrently with migrate/sync-metadata — I confirmed both are read-only (no INSERT/UPDATE/DELETE), so no corruption (SQLite WAL), but a destructive migration mid-deploy can fire a spurious health-check failure alert; (b) more importantly, the gate's own stated purpose — "the next pipeline/decimate/status run would execute the old checkout against the migrated DB" — is exactly what kayak-recap would do after a cutover, undetected, because it isn't on the list. Recommend the default KAYAK_UNITS (and deploy.env.example) enumerate every engine/DB-touching unit, or derive the consumer set instead of hardcoding; at minimum make the 4C runbook responsible for a complete list. (KAYAK_UNITS is overridable, so this is about the default being a trustworthy safety net.)

F2 — Low (latent, conditional on root umask): the umask-077 defense is incomplete.
The script chmod 0755s only $KAYAK_DEPLOY_TMPDIR (.staging), explicitly because "root umask may be 077" and the app user must traverse into the app-owned scratch. But $ROOT (/opt/kayak), $ROOT/releases, and $ROOT/releases/<id> are created with mkdir -p under the deployer's umask and are never chmod'd — yet the app user must traverse $ROOT → releases → <id> to write docroot (the app-user build) and $ROOT → .staging → app.XXX for .backup. On a host whose root umask is 077 (the exact case the .staging chmod defends against), /opt/kayak is created 0700 and every app-user step fails with permission denied. Debian's default root umask is 022, so this is latent today — but the partial defense is inconsistent. Either the umask isn't a concern (drop the .staging chmod) or it is (also chmod 0755 "$ROOT" "$ROOT/releases", or set umask 022 at the top of the script).

F3 — Low (divergent-parser footgun): the deployer shell-sources the app user's ~/.config/kayak/.env.
That file is authored for systemd EnvironmentFile= and python-dotenv semantics (literal KEY=value, no shell quoting required). The deployer now .-sources it under set -euo pipefail, which is shell-parsed. It works today only because the one multi-word value (EXPECTED_SANS="a b c") happens to be quoted. A future edit adding an unquoted multi-word value, a bare $/backtick, or a mid-line # would break the deploy at config-load (and could execute words) while leaving systemd and PHP unaffected — a failure that appears only on the deploy path. deploy.env.example warns about quoting for deploy.env itself; the app .env it newly co-opts gets no such guarantee. Prefer a non-shell reader, or validate/document the quoting requirement on the app .env.

F4 — Nit: the host-config fingerprint is inert on this host. /etc/kayak/host.yaml doesn't exist, so HOST_FP="none" always. The headline "a host-config-only change is a distinct release" property is dormant until host.yaml is actually deployed (PR #189 added the typed config; the file isn't installed). Handled gracefully — just don't assume that identity guarantee is live yet.

F5 — Nit: HEALTH_URL is optional. With it unset, activation flips the symlink with no post-switch health gate (the rollback-on-failed-health path never runs). deploy.env.example suggests https://levels.wkcc.org/status.json — recommend the 4C runbook treat it as required for the first real cutover.

F6 — Nit/operational: a deploy overlapping the hourly *:12 pipeline terminates the in-flight run. Pipeline runs measure ~110–116 s steadily on this host; kayak-pipeline.service is Type=oneshot, KillMode=control-group, TimeoutStopUSec=90 s. So systemctl stop mid-run kills the cycle (SIGTERM→SIGKILL ≤90 s; data-safe under WAL), and the 120 s drain backstop sits just above the 90 s stop timeout — sized fine. Just schedule deploys clear of the :12 window so you don't routinely abort a running fetch.


Bottom line: solid, genuinely well-hardened work — the inert-until-cutover gating means this merges with no risk to the live site. F1 is the one I'd want addressed (or explicitly delegated to the 4C runbook) before the first real activation, since the consumer set is what stands between a migrate/sync and old code reading the post-migration DB. F2–F3 are latent footguns worth a cheap fix; F4–F6 are notes for the cutover runbook.

Reviewed against the live WKCC host on 2026-06-13. 🤖 Generated with Claude Code

…k, units)

Two live-server adversarial reviews of a22585b (codex/gpt-5.5 + claude/
opus). Both: no merge blocker (inert until SERVING_CUTOVER=yes). These
are the real code-level findings, fixed before merge:

- SECURITY (codex P1 / claude F3): the deployer no longer shell-`source`s
  the app-user-owned ~/.config/kayak/.env as root — that was a root
  code-execution + deploy-control-override surface (it could set
  ENGINE_REPO, KAYAK_SYSTEMCTL, …) introduced in be43a93. Read only the
  allowlisted data keys (SITE_URL/SQLITE_PATH/DATASET_DIR/OUTPUT_DIR) as
  data, only when the root-owned env hasn't already set them. /etc/kayak/
  env (root-owned) is still sourced.

- reuse READ-ONLY (codex P2): the reuse branch no longer rm's +
  re-extracts the (possibly live) release's dataset before the stage-only
  exit / quiesce — it diff-verifies the on-disk dataset against the tar
  from scratch and fails closed on mismatch. Upholds "stage-only stops
  before mutation."

- umask 022 (claude F2): consistent dir perms so $ROOT/releases/<id> stays
  app-user-traversable even under a root umask of 077 (the .staging chmod
  alone was inconsistent).

- kayak-healthcheck.timer added to the default KAYAK_UNITS + example (a
  live DB consumer the cutover gate couldn't see); the 4C runbook owns the
  COMPLETE enumeration.

Tests: new test_app_env_keys_read_as_data_not_sourced (a malicious app
.env line must not execute, the allowlisted key is read as data, a
non-allowlisted deploy control can't override); a reuse-is-read-only
assertion; and the slow activation tests stage on a real-disk root
(/var/tmp) so the module is runnable on a prod-shaped host (964 MB /tmp
tmpfs) — codex couldn't run it before. `_run` gained None=remove-key for
hermetic envs.

Deferred to the 4C runbook (cutover-time): the robust consumer-set
derivation (recap/heartbeat/config-drift), HEALTH_URL required for the
first cutover, and scheduling clear of the :12 pipeline.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mousebrains

Copy link
Copy Markdown
Owner Author

Both live-server reviews addressed in 530b4a0. Both concluded "no merge blocker" (inert until SERVING_CUTOVER=yes); the items below are the real code-level ones, fixed before merge — the rest are cutover-time and go to the 4C runbook.

Fixed in 530b4a0

  • 🔴 app .env sourced as root (codex P1 / claude F3). The deployer no longer sources the app-user-owned ~/.config/kayak/.env as root — that was the root code-execution + deploy-control-override surface I introduced in be43a93 to make SITE_URL/SQLITE_PATH resolve. It now reads only the allowlisted data keys (SITE_URL/SQLITE_PATH/DATASET_DIR/OUTPUT_DIR), as data (never evaluated), and only when the root-owned env hasn't already set them. /etc/kayak/env (root-owned) is still sourced. New test asserts a malicious line in the app .env does not execute, the allowlisted key is read, and a non-allowlisted key can't override a deploy control.

  • 🟡 reuse mutates a (possibly live) release during --stage-only (codex P2). The reuse branch no longer rms + re-extracts the release's dataset before the stage-only exit/quiesce — it diff-verifies the on-disk dataset against the tar from scratch and fails closed on mismatch. The release dir is never written during staging. Test asserts the dataset survives a same-refs reuse.

  • 🟢 umask 022 (claude F2). Makes $ROOT/releases/<id> app-user-traversable under a root umask of 077 too — the .staging chmod alone was an inconsistent defense.

  • kayak-healthcheck.timer → default KAYAK_UNITS + example (codex P1#2 / claude F1, partial). It reads the DB and is active on the live host. The complete enumeration (recap/heartbeat/config-drift run the old checkout too) + deriving the set from installed timers is a 4C item — see below.

  • slow tests runnable on a prod-shaped host (codex verification). The activation tests now stage on a real-disk root (/var/tmp, override KAYAK_TEST_DEPLOY_TMPDIR) instead of pytest's /tmp tmpfs, so the full module runs where the live /tmp is a 964 MB tmpfs.

Deferred to the 4C runbook / PR (cutover-time, as both reviews suggested)

  • claude F1 (robust): derive the consumer set from installed kayak-* timers / enumerate every engine+DB consumer; the runbook owns a complete, host-verified list.
  • claude F5: treat HEALTH_URL as required for the first real cutover (otherwise no post-switch health gate).
  • claude F6: schedule deploys clear of the :12 pipeline window.
  • claude F4: host.yaml fingerprint is dormant until the file is installed — noted, not assumed live.

Gate: ruff/format/shellcheck/mypy clean; 1725 fast tests + the slow deploy suite (stage/rollback/prune on a real-disk root) green.

@mousebrains

Copy link
Copy Markdown
Owner Author

Re-review of 530b4a0 — fixes verified against the live host

Checked the hardening commit against the running WKCC host. All four changes hold up, and the one finding that mattered is fully closed.

✅ F3 / codex P1 (the security one) — closed and verified RCE-safe. Replacing the . "$APP_ENV" shell-source with the allowlisted data-reader was the right call (and codex's escalation to a root code-exec / deploy-control-override surface is correct — a source of the app-user-owned file as root could set ENGINE_REPO, KAYAK_SYSTEMCTL, etc.). I exercised the new loop adversarially:

  • A SITE_URL=$(touch marker) line and a bare `touch marker` line did not execute — no marker files created.
  • Allowlisted keys are read as data with quote-stripping; a non-allowlisted ENGINE_REPO= in the app .env cannot override the deploy.env value.
  • Against the real ~/.config/kayak/.env it extracts exactly: SITE_URL=https://levels.wkcc.org, SQLITE_PATH=/home/pat/DB/kayak.db, DATASET_DIR=/home/pat/kayak_data, OUTPUT_DIR=/home/pat/public_html. The allowlist (those 4) is precisely the set the deployer needs; the now-unread HC_*/NTFY_TOPIC/USGS_API_KEY were being normalized/excluded out of the staged config anyway, and the canonical PHP secrets still come from /etc/kayak/secrets.env via the root wrapper, so nothing downstream regresses.

✅ F2 (umask) — fixed correctly. umask 022 up front makes $ROOT/releases/<id> 0755 and app-user-traversable even under a root umask of 077, which is the consistent version of the partial .staging chmod. No secret-leak concern: release dirs hold only non-secret content (wheel/lock/non-secret config/venv/dataset/docroot); emit-config still chmod 0640s its own output explicitly, and mktemp -d stays 0700 regardless of umask.

✅ codex P2 (reuse read-only) — good catch & fix. The old rm -rf $RELEASE_DIR/dataset + re-extract ran before the stage-only exit, so a same-refs rehearsal against the currently-active release could have yanked the live dataset out from under running consumers. Diff-verifying a scratch extract against the on-disk dataset (fail-closed on mismatch, release dir never written) restores the "stage-only mutates nothing" contract. Combined with the manifest tar-digest check, on-disk == tar == manifest transitively.

◐ F1 (consumer set) — highest-value piece taken, rest appropriately deferred. kayak-healthcheck.timer (the frequent live DB reader) is now in the default KAYAK_UNITS, and deploy.env.example warns that the list gates the cutover check. Confirmed still-outstanding on this host and explicitly deferred to the 4C runbook: kayak-heartbeat (daily, reads ~/DB/kayak.db) and kayak-recap/kayak-config-drift (run engine code from the old /home/pat/kayak checkout). Those are read-only / non-DB, so no corruption risk in the deploy window — but recap running old code post-cutover is exactly the gate's stated concern, so the 4C runbook must enumerate the complete set (the comment commits to deriving it from installed kayak-* timers there). Fine to defer with that written commitment.

Notes I'm satisfied are covered, not gaps: SITE_URL flowing from the app-env allowlist → run_app/runuser → the activation emit-config rides the deployer's exported env, the same propagation the activation tests already exercise via KAYAK_HOST_ENV; the unit test covers the data-read up through the staged config. F4 (host-config fingerprint inert — no /etc/kayak/host.yaml), F5 (HEALTH_URL required for first cutover), and F6 (schedule clear of the :12 pipeline) remain runbook items, as noted in the commit.

Bottom line: the real finding (root source-injection) is closed and I've verified it against this host's actual .env; the rest are correct. No blockers — mergeStateStatus is CLEAN. The remaining consumer-set enumeration is the one thing the 4C cutover runbook must not skip.

Re-reviewed against the live WKCC host, 2026-06-13. 🤖 Generated with Claude Code

@mousebrains
mousebrains merged commit cf33980 into main Jun 13, 2026
9 checks passed
@mousebrains
mousebrains deleted the b4-paired-release branch June 13, 2026 23:01
mousebrains added a commit that referenced this pull request Jun 14, 2026
…lure (errtrace)

Adversarial review of #192 surfaced a chain of rollback gaps. Fixing finding #1
(arm DOCROOT_BUILT before the build) exposed a deeper, pre-existing PR #190 bug:
the rollback never ran for the activation mutation steps at all.

- **errtrace (the big one).** The deployer ran `set -euo pipefail` without `-E`.
  Every DB/build mutation runs through `run_app()` (a function), and without
  errtrace a failure INSIDE a function aborts under `set -e` WITHOUT triggering
  the `ERR` trap. So a failed `run_app … migrate/sync/import/build` exited with
  the DB already mutated, consumers stopped, maintenance still on — and NO
  rollback (only the EXIT scratch-cleanup). `set -Eeuo pipefail` makes those
  failures hit `trap rollback ERR`. The trap is armed for the activation phase
  only (staging has no ERR trap, so `-E` is inert there); atomic_relink's `mv`
  probes sit in `if` conditions, which stay exempt. Surfaced writing finding #1's
  test — the docroot-rebuild fix is moot if rollback doesn't run.

- **#1: DOCROOT_BUILT armed before the build, not after.** `build` writes the
  live docroot in place (stage → per-file rename → orphan sweep), so a failure
  mid-write leaves it mixed yet exits non-zero. The flag must mean "build
  started" (docroot may have changed) so rollback rebuilds; a rebuild when it was
  in fact untouched is harmless/idempotent.

- **#3: normalize an absolute `current` symlink target.** The rebuild used
  `$ROOT/$PREV_TARGET`; a manual-recovery absolute `current` made that
  `/opt/kayak//opt/...`. Compute `PREV_DIR` with an absolute/relative case split.

- **nit #4: keep the rollback rebuild's stderr** (to `$SCRATCH/docroot-rebuild.err`,
  retained) instead of discarding it — the recovery path is where the reason
  matters most.

- **#2 (ACL): commit the shared-docroot web ACL** to deploy/SETUP.md's cutover
  section — the recursive + default `u:www-data:rX` on `/var/cache/kayak/docroot`
  the deployer doesn't grant; without the `-d` default ACL each freshly built
  page 403s. The nginx-root / FPM-open_basedir / unit verification gate stays a
  4C-renderer deliverable (it owns those templates).

Tests: two new slow tests — build mutates the docroot then exits 1 → rollback
rebuilds (proves the flag-before-build + errtrace fix); and an absolute `current`
target → rebuild reads the normalized dir. Full slow suite 5 passed, non-slow
1725 passed, ruff/format/shellcheck clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mousebrains added a commit that referenced this pull request Jul 15, 2026
)

This is what actually retires #228. `[build-system].requires` was unbounded,
requirements-build.lock is `uv pip compile`d from it, and CI's drift check
recompiles against live PyPI every run and diffs — so hatchling 1.31.0
shipping reddened every open PR, whatever its diff. #228 regenerated the lock
by hand; that bought time, it didn't fix anything.

A range does not fix it either, which is worth stating because it is the
obvious reach: `uv pip compile` resolves the NEWEST version a specifier
admits, so `>=1.27,<2` re-resolves the day 1.32.0 lands and the lock is stale
again. Measured, not assumed:

    hatchling>=1.27,<2      -> 1.31.0    (newest in range — drifts again)
    hatchling>=1.27,<1.31   -> 1.30.1    (proves it takes the max, not a pin)
    hatchling==1.30.1       -> 1.30.1    (stable)

Only `==` makes compile-then-diff idempotent, which is the property the check
needs. It also fits what this feeds: kayak-deploy builds the wheel with
--no-build-isolation from the hashed lock (#190), so the backend version is
part of the artifact and pinning it is the point, not a wart.

The cost is that nothing watches this line — no Dependabot ecosystem reads
[build-system].requires, so hatchling now goes stale until bumped by hand. A
stale-but-working build backend is a strictly better failure than an
unpredictable red CI, and the comment carries the two-line bump recipe.

Lock unchanged: #228 already put 1.31.0 there, so this only removes the
mechanism that made that a recurring chore. Drift check verified idempotent
across repeated runs; wheel builds clean.


Claude-Session: https://claude.ai/code/session_01RR6LwVdry9fmXkEhkC9Cbi

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant